Skip to content

fix: Allow filtering null values #1067

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Feb 17, 2023

Conversation

danieljbruce
Copy link
Contributor

@danieljbruce danieljbruce commented Feb 15, 2023

A user expressed interest in filtering with a null value. Currently this is possible if a null value is provided in a variable, but if a null literal is passed into value for .filter then we get a typescript compiler error. This typescript parameter change will now allow users to not experience the typescript compiler error when trying to filter by null.

Fixes #958

A user expressed interest in filtering with a null value. This typescript parameter change will now allow users to do so.

fixes googleapis#958
@danieljbruce danieljbruce requested review from a team as code owners February 15, 2023 22:24
@product-auto-label product-auto-label bot added size: s Pull request size is small. api: datastore Issues related to the googleapis/nodejs-datastore API. labels Feb 15, 2023
The code in the system test folder reflects the user experience more closely so we want to provide test cases there that will break when compiling typescript if new changes in src are not provided.
Copy link
Contributor

@kolea2 kolea2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but please:

  1. rename the PR to be clear this is for filtering (maybe something like 'fix: allow filtering null values')
  2. Please have someone from yoshi-nodejs take a look

@danieljbruce danieljbruce changed the title fix: Allows nulls to be passed into value fix: Allow filtering null values Feb 16, 2023
Add assert statements to check that the value in the filter created in the system test actually equals null.
Slight rename in system tests. Modified tests so that they test to see that the filter has a null value.
@danieljbruce danieljbruce added the owlbot:run Add this label to trigger the Owlbot post processor. label Feb 16, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Feb 16, 2023
This change replaces the a longer fragment with elvis operator to make assert statements more concise.
In two tests we eliminate variables and just have an assert statement so that we can have more concise code.
@danieljbruce danieljbruce added the owlbot:run Add this label to trigger the Owlbot post processor. label Feb 16, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Feb 16, 2023
Copy link
Contributor

@ruyadorno ruyadorno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@danieljbruce danieljbruce added the owlbot:run Add this label to trigger the Owlbot post processor. label Feb 17, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Feb 17, 2023
@danieljbruce danieljbruce merged commit b89fa21 into googleapis:main Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: datastore Issues related to the googleapis/nodejs-datastore API. size: s Pull request size is small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to filter property with null value
3 participants